exec command #27
exec command #27
Conversation
ksimon1
force-pushed
the
run-command-command
branch
from
7b07899
to
24aa035
Compare
cmd/exec.go
Outdated
| return err | ||
| } | ||
|
|
||
| return hnd.Exec(containerID, []string{execOpt.command}, os.Stdout) |
There was a problem hiding this comment.
...so this slice will always have len == 1
Is that what you want?
ksimon1
force-pushed
the
run-command-command
branch
2 times, most recently
from
56497bd
to
ddc200c
Compare
| exec := &cobra.Command{ | ||
| Use: "exec", | ||
| Short: "exec runs given command in container", | ||
| RunE: execCommand, |
There was a problem hiding this comment.
I'd use
Args: cobra.MinimumNArgs(1),
ddc200c
to
6e1a964
Compare
There was a problem hiding this comment.
LGTM. Looking forward to learn how you want to use this.
We most likely need to update kubevirt/kubevirtci#183
| @@ -64,6 +65,7 @@ func NewRunCommand() *cobra.Command { | |||
| run.Flags().UintVar(&okdRunOpts.sshWorkerPort, "ssh-worker-port", 0, "port on localhost to ssh to worker node") | |||
| run.Flags().BoolVar(&okdRunOpts.background, "background", false, "go to background after nodes are up") | |||
| run.Flags().BoolVar(&okdRunOpts.randomPorts, "random-ports", true, "expose all ports on random localhost ports") | |||
| run.Flags().StringVar(&okdRunOpts.volume, "volume", "", "Bind mount a volume into the container") | |||
There was a problem hiding this comment.
Will be the volume automatically unbound? Do we need some cleanup?
There was a problem hiding this comment.
According to this: https://github.com/fromanirh/pack8s/blob/master/cmd/rm.go#L87
all builtin volumes should be removed with container (https://github.com/containers/libpod/blob/e7540d0406c49b22de245246d16ebc6e1778df37/cmd/podman/varlink/io.podman.varlink#L733)
There was a problem hiding this comment.
This is fine. But does this also mean that the more exec we do on a container, the more volumes we pile in? what does it happen if we do two pack8s exec one after the other? will it work? will it pile up temporary volumes in the container?
Each call of pack8s exec must be idenpotent and do proper cleanup after itself; is not OK to rely on external tools (e.g. kubevirtci scripts) to do cleaning, because it's fragile and leads to surprising (and most likely unwanted) results.
There was a problem hiding this comment.
The volume is bind during start of container and persist till stop of container. It will not add or remove any volume during exec. I agree pack8s should handle cleanup and as i mentioned in previous comment, it removes all binded volumes with rm command
There was a problem hiding this comment.
I'm not 100% sure it's a good idea to have this volume bound between host and container, but since this is strictly for development and we are in the early stage of development, I'm incline to try it out and see how it looks.
cmd/exec.go
Outdated
| return err | ||
| } | ||
|
|
||
| return hnd.Exec(containerID, execOpt.commands, os.Stdout) |
There was a problem hiding this comment.
now that I see how you want to use this, I think --commands is bad UX.
First, it should be --command (singular), because exec can run just a single command - not more than one.
Second, we should not require it to be comma separated. It should be a regular shell line. The simplest way is probably to pass through args from line 31 in hnd.Exec() here on line 46, perhaps with some minimal massaging.
This can improved later, but we should have it before submitting the kubevirtci PR.
| @@ -64,6 +65,7 @@ func NewRunCommand() *cobra.Command { | |||
| run.Flags().UintVar(&okdRunOpts.sshWorkerPort, "ssh-worker-port", 0, "port on localhost to ssh to worker node") | |||
| run.Flags().BoolVar(&okdRunOpts.background, "background", false, "go to background after nodes are up") | |||
| run.Flags().BoolVar(&okdRunOpts.randomPorts, "random-ports", true, "expose all ports on random localhost ports") | |||
| run.Flags().StringVar(&okdRunOpts.volume, "volume", "", "Bind mount a volume into the container") | |||
this command can run user commands inside container. This feature will be used in kubevirtci to copy file within container into speacial folder which is mounted at the same time in container and host. With this we can copy files from container to host, without any hacks. Signed-off-by: Karel Šimon <ksimon@redhat.com>
ksimon1
force-pushed
the
run-command-command
branch
from
6e1a964
to
544f80a
Compare
|
I was expecting a flow like but actually adding a utility volume to the container is viable too. Let's try this out. |
this command can run user commands inside container.
We need this command, because kubevirtci needs to copy some files from container to host. And with this, we can copy these files to folder which is mounted in host and in container.
/cc @fromanirh
Signed-off-by: Karel Šimon ksimon@redhat.com